Thank you! But won't this exact case already be caught by the expression below?

The difference is that the query param has to have select *, this is being interpreted by the query match pattern %Ecto.Query{select: %SelectExpr{expr: {:&, [], [0]}}}

I think this AST {:&, [], [0]} means user is not trying to select specific fields
Thanks for your review

Yes, but it will already raise with the error message below. Perhaps we should change the error below instead to say use distinct: true instead?

Ecto actually selects the fields from the schema individually. So even if you don’t specify the fields yourself Ecto will. It won’t do select *.

Ah sorry i got it now you mean this case

defp distinct(%ByExpr{expr: exprs}, _sources, query) when is_list(exprs) do
      error!(query, "DISTINCT with multiple columns is not supported by MySQL")
end

Why i didnt think about it because in sql you cant do something like this anyway:
SELECT col1,col2 DISTINCT col1, col2
but you can do this
SELECT DISTINCT col1, col2

First case equivalent to distinct(%ByExpr{expr: exprs}, _sources, query) when is_list(exprs)
Second equivalent to the new one with the SelectExpr pattern match which should work in Mysql

What do you think? should i delete new pattern match and simply just change the below case msg error
or make ecto works with the new case with new pattern match like its working in Mysql?

@greg-rychlewski Thanks for the clarification

Sorry, to clarify, I am saying that, instead of adding a whole new clause, you could update the existing one to add more information. There is no reason to have two very similar code paths. :)

Done 🫡